Skip to content

Fix Pass data and country package versions to APIv2 #2500#2528

Merged
nikhilwoodruff merged 7 commits into
masterfrom
data-model-versions
Jun 4, 2025
Merged

Fix Pass data and country package versions to APIv2 #2500#2528
nikhilwoodruff merged 7 commits into
masterfrom
data-model-versions

Conversation

@nikhilwoodruff

Copy link
Copy Markdown
Collaborator

Fixes #2500

@nikhilwoodruff nikhilwoodruff self-assigned this Jun 2, 2025
@nikhilwoodruff nikhilwoodruff requested a review from anth-volk June 2, 2025 11:31
@codecov

codecov Bot commented Jun 2, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 66.07143% with 19 lines in your changes missing coverage. Please review.

Project coverage is 63.75%. Comparing base (a1c0a0e) to head (2653504).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
policyengine_api/utils/hugging_face.py 62.79% 11 Missing and 5 partials ⚠️
...ngine_api/jobs/calculate_economy_simulation_job.py 76.92% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2528      +/-   ##
==========================================
+ Coverage   63.62%   63.75%   +0.13%     
==========================================
  Files          56       57       +1     
  Lines        2161     2213      +52     
  Branches      287      297      +10     
==========================================
+ Hits         1375     1411      +36     
- Misses        729      740      +11     
- Partials       57       62       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nikhilwoodruff nikhilwoodruff added the enhancement New feature or request label Jun 2, 2025
Comment thread policyengine_api/jobs/calculate_economy_simulation_job.py
@nikhilwoodruff nikhilwoodruff requested a review from anth-volk June 3, 2025 11:18
Comment on lines +225 to +229
data_version=(
uk_dataset_version
if country_id == "uk"
else us_dataset_version
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue, blocking: This code is HF-dependent and integrated into API v2

I really do not think we should be adding any code that hitches API v2 to Hugging Face. I think all of the following would be acceptable and a means of getting this over the line:

  1. Agree that we'll remove dataset version checks after fully migrating (is this a good idea?)
  2. Agree that we'll write code in a separate PR to check dataset versions within GCP before full migration (probably my preferred way forward)
  3. Switch to using GCP in API v1 now and write code to check dataset versions from GCP

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved: We'll go down pathway 1, removing dataset version checks after fully migrating.

@anth-volk anth-volk self-requested a review June 3, 2025 23:02

@anth-volk anth-volk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Holding off on merging until June 4, following completion of regression tests on June 3.

Comment on lines +225 to +229
data_version=(
uk_dataset_version
if country_id == "uk"
else us_dataset_version
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved: We'll go down pathway 1, removing dataset version checks after fully migrating.

@nikhilwoodruff nikhilwoodruff merged commit d533848 into master Jun 4, 2025
7 checks passed
@nikhilwoodruff nikhilwoodruff deleted the data-model-versions branch June 4, 2025 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pass data and country package versions to APIv2

2 participants